-
Notifications
You must be signed in to change notification settings - Fork 21
(FM-2577) Minor SQL server connection building refactorings #105
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
(FM-2577) Minor SQL server connection building refactorings #105
Conversation
- Remove erb template and the code that calls it. Should have been removed in a prior commit, but was accidentally left over.
Waiting for Travis-CI but 👍 |
params = { | ||
'Provider' => 'SQLOLEDB.1', | ||
'User ID' => config['admin'], | ||
'Password' => config['pass'], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing Persist Security Info and tests failing as a result.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Gotcha - will fixup test. Note that I removed Persist Security Info
since it has been the default for a long time.
a89119f
to
ca24c1c
Compare
- Note that Persist Security Info = False was removed from the Connection String since it's already the default.
ca24c1c
to
01e6cb6
Compare
…lcmd-remnants (FM-2577) Minor SQL server connection building refactorings
config = {'database' => 'master'}.merge(config) | ||
# Open ADO connection to the SQL Server database | ||
connection_string = "Provider=SQLOLEDB.1;" | ||
connection_string << "Persist Security Info=False;" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this line was lost, never to be found again. @Iristyle @cyberious was that intentional?
No description provided.